Compressed offload support for IPC4 #10492

ranj063 · 2026-01-21T04:26:20Z

This PR is part 1 of the compressed offload support which includes splitting up the cadence implementation for IPC3 and IPC4 along with the required patches for setting a DP module's ring buffer size based on what the input buffer size requirements for the codec are and the init data required for module init. The topology patches showing the compressed path will be included in the next part

Copilot

Pull request overview

This PR adds compressed offload support for IPC4 by refactoring the Cadence codec implementation. It splits the existing implementation into IPC3 and IPC4-specific variants while keeping common functionality shared.

Changes:

Split Cadence codec implementation into IPC3 (cadence_ipc3.c) and IPC4 (cadence_ipc4.c) specific files
Modified ring buffer creation to account for module-specific buffer size requirements
Added support for module initialization data in IPC4

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/ipc/ipc4/helper.c	Updated ring buffer size calculation to use MAX of default sizes and module-specific buffer requirements
src/include/sof/audio/module_adapter/module/cadence.h	Exposed common Cadence codec functions, added IPC4-specific structures, moved API ID enum from .c file
src/include/sof/audio/cadence/mp3_enc/xa_mp3_enc_api.h	Added new MP3 encoder API header with configuration parameters and error codes
src/include/ipc4/module.h	Added MODULE_DATA initialization data type to support codec-specific init data
src/audio/module_adapter/module_adapter_ipc4.c	Added handling for MODULE_DATA initialization data type
src/audio/module_adapter/module/cadence_ipc4.c	New IPC4-specific Cadence codec implementation with DP-aware processing
src/audio/module_adapter/module/cadence_ipc3.c	Extracted IPC3-specific Cadence codec implementation
src/audio/module_adapter/module/cadence.c	Refactored to contain only common codec functionality shared between IPC3/IPC4
src/audio/module_adapter/CMakeLists.txt	Updated build configuration to compile IPC-specific implementations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/audio/module_adapter/module/cadence_ipc4.c

Copilot · 2026-01-21T04:27:15Z

src/audio/module_adapter/module/cadence_ipc4.c

+{
+	struct cadence_codec_data *cd = module_get_private_data(mod);
+	struct comp_dev *dev = mod->dev;
+	int word_size = 16;


The hardcoded word size value 16 is used here but then validated against audio_fmt.depth in the switch statement below. Consider using a constant definition or deriving the value from the configuration to make the relationship clearer.

ranj063 · 2026-01-21T17:11:59Z

@dbaluta would you be able to give this PR a try to make sure IPC3 compressed support is still good? On the kernel side, the split was cleaner but I had to retain the common parts in this PR for IPC3 and IPC4. Thanks!

dbaluta · 2026-01-22T11:45:54Z

@ranj063 I get the following compile time errors:

home/daniel/work/sof_dir4/sof/src/audio/module_adapter/module/cadence_ipc3.c: In function 'cadence_codec_resolve_api':
/home/daniel/work/sof_dir4/sof/src/audio/module_adapter/module/cadence_ipc3.c:25:26: error: storage size of 'codec_params' isn't known
   25 |         struct snd_codec codec_params;                                                                                                                                                
      |                          ^~~~~~~~~~~~                                                                                                                                                 
/home/daniel/work/sof_dir4/sof/src/audio/module_adapter/module/cadence_ipc3.c:25:26: warning: unused variable 'codec_params' [-Wunused-variable]
/home/daniel/work/sof_dir4/sof/src/audio/module_adapter/module/cadence_ipc3.c: In function 'cadence_codec_process':
/home/daniel/work/sof_dir4/sof/src/audio/module_adapter/module/cadence_ipc3.c:221:36: warning: unused variable 'cd' [-Wunused-variable]
  221 |         struct cadence_codec_data *cd = codec->private;
      |                                    ^~
[174/335] Building C object modules/sof/CMakeFiles/modules_sof.dir/home/daniel/work/sof_dir4/sof/src/audio/selector/selector.c.obj

Looks like you need to add the following fix:

diff --git a/src/audio/module_adapter/module/cadence_ipc3.c b/src/audio/module_adapter/module/cadence_ipc3.c
index 2bafe766d..77b5b0276 100644
--- a/src/audio/module_adapter/module/cadence_ipc3.c
+++ b/src/audio/module_adapter/module/cadence_ipc3.c
@@ -11,6 +11,7 @@
  *
  */
 
+#include <ipc/compress_params.h>
 #include <sof/audio/module_adapter/module/generic.h>
 #include <sof/audio/module_adapter/module/cadence.h>
 #include <rtos/init.h>
@@ -218,7 +219,6 @@ cadence_codec_process(struct processing_module *mod,
        struct comp_buffer *local_buff;
        struct comp_dev *dev = mod->dev;
        struct module_data *codec = &mod->priv;
-       struct cadence_codec_data *cd = codec->private;
        int free_bytes, output_bytes = cadence_codec_get_samples(mod) *
                                mod->stream_params->sample_container_bytes *
                                mod->stream_params->channels;

Otherwise it works fine.

wjablon1 · 2026-01-22T13:02:51Z

src/audio/module_adapter/module_adapter_ipc4.c

+		{
+			/* set the module init_data */
+			dst->init_data = (const void *)(obj + 1);
+			dst->avail = true;


Did you verify that dst->avail is set to false before the function call (e.g. through memset)? I.e are we prepared for a situation where item "IPC4_MOD_INIT_DATA_ID_MODULE_DATA" is not present for some reason?

@wjablon1 module_ext_init_decode() is called right after we receive the module init IPC for a module. And when we receive the IPC, "mod" data for the module is allocated right then and it is cleared to 0 with a memset. Is that what you're asking?

yep. If mod->priv.cfg is memseted to zeros prior to module_adapter_init_data, I don't see an issue here. Otherwise I would manually set dst->avail to false before module_ext_init_decode.

lyakh · 2026-01-21T08:26:57Z

src/audio/module_adapter/module_adapter_ipc4.c

+		case IPC4_MOD_INIT_DATA_ID_MODULE_DATA:
+		{
+			/* set the module init_data */
+			dst->init_data = (const void *)(obj + 1);


isn't the obj pointer in the IPC buffer, which will be overwritten with the next IPC?

This also caught my attention but then I noticed that the init_data pointer is set back to NULL after initialization in module_adapter_reset_data. Probably we should also set dst->avail to false in the module_adapter_reset_data.

@lyakh the idea for this is to just pass the init data pointer to the module-specific init and it is up to the module to take it or not. For example, in the case of the cadence decoder, we save this data which contains the codec params requested by the kernel like here

sof/src/audio/module_adapter/module/cadence_ipc3.c

Line 76 in 721418e

ret = memcpy_s(setup_cfg->data, setup_cfg->size,

we should probably call this out in the comments that clients must copy if they need to retain the data after IPC

...or maybe we can move that = NULL into this or the calling function and remove module_adapter_reset_data() completely

My votes to set the init_data pointer to NULL after module init is complete and we return from IPC handling. If this cannot be done cleanly, then I think we should do a copy of the data. Otherwise this is like a trap waiting to spring for module developers.

another possibility would be passing it as a separate argument to whatever functions need it without assigning it to persistent objects

@kv2019i As I mentioned the pointer is already set to NULL after module initialization in module_adapter_reset_data. I kind of agree with @lyakh. Some form of separation is needed. Either a separate function for unpacking temporary data or a separate destination data structure or maybe some sub-structure of module_config (like dst->tmp_data.init_data).

I'm going to take this as-is today but with added doxygen and inline comment stating the need to copy as there is a lot on in progress work around userspace copying of IPC data and this would also benefit by reusing the same. i.e. there is little value in implementing a solution here today when we will need to change it in a few weeks for userspace.

lgirdwood

just some minor opens from me.

lgirdwood · 2026-01-23T17:02:33Z

src/audio/module_adapter/module_adapter_ipc4.c

+		case IPC4_MOD_INIT_DATA_ID_MODULE_DATA:
+		{
+			/* set the module init_data */
+			dst->init_data = (const void *)(obj + 1);


we should probably call this out in the comments that clients must copy if they need to retain the data after IPC

src/audio/module_adapter/module/cadence_ipc4.c

kv2019i

Nothing major, but a few questions inline.

kv2019i · 2026-01-26T09:00:32Z

src/audio/module_adapter/module_adapter_ipc4.c

+		case IPC4_MOD_INIT_DATA_ID_MODULE_DATA:
+		{
+			/* set the module init_data */
+			dst->init_data = (const void *)(obj + 1);


My votes to set the init_data pointer to NULL after module init is complete and we return from IPC handling. If this cannot be done cleanly, then I think we should do a copy of the data. Otherwise this is like a trap waiting to spring for module developers.

kv2019i · 2026-01-26T09:21:31Z

src/ipc/ipc4/helper.c

+		ring_buffer = ring_buffer_create(dp, MAX(source_get_min_available(src),
+							 dst_module_data->mpd.in_buff_size),
+						 MAX(sink_get_min_free_space(snk),
+						     src_module_data->mpd.out_buff_size),


Could you explain why this is needed, perhaps with some example? I thought module should advertise its requirements via the sink/source API, so tha tsource_get_min_available() and sink_get_min_free_space() would require bigger values. Directly accessing mpd.in/out_buff_size seems a bit like punching holes through the sink/source (see logic above on L623 and above). I could be missing something, but would be good to record the logic a bit more in the git commit to help the next person that needs to modify this.

@kv2019i an example is the cadence decoder in the following path that needs this. We do not know what the min size of the input buffer needs to be for this module. We typically use the ibs/obs to calculate this but for the decoder module, the ibs/obs is sort of iirelevant and we typically get the minimum required samples using the cadence API. So this is what it does here.

src/audio/module_adapter/module/cadence_ipc4.c

Add a global ID for module specific init data in the extended init data section and update the decode function to save the data in the module's init_data. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

When a module advertises the input and output buffer requirements for processing, use that to determine the size of the secondary buffer when attaching them to buffers interfacing with DP modules. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 · 2026-01-27T16:37:39Z

@lyakh @lgirdwood @kv2019i I have updated the PR to add comments for clarification now. Please take alook

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/audio/module_adapter/module/cadence_ipc4.c

Split the implementation for IPC3 for the cadence decoder and add a new implementation for IPC4. The IPC4 implementation assumes that the codec info is passed to the firmware during the module init IPC as module specific data. The implementation includes support for MP3 decoder, MP3 encoder and AAC decoder for the time being. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 · 2026-01-27T16:55:17Z

@ranj063 I get the following compile time errors:

@dbaluta I have fixed the compilation errors with IPC3.

Copilot AI review requested due to automatic review settings January 21, 2026 04:26

ranj063 requested review from RanderWang, dbaluta, jxstelter, kv2019i, lbetlej, lgirdwood, marcinszkudlinski, mmaka1, pblaszko and plbossart as code owners January 21, 2026 04:26

Copilot AI reviewed Jan 21, 2026

View reviewed changes

wjablon1 reviewed Jan 22, 2026

View reviewed changes

ranj063 force-pushed the topic/ipc4_compress_part1 branch from d137da9 to 721418e Compare January 22, 2026 16:51

lyakh reviewed Jan 23, 2026

View reviewed changes

lgirdwood reviewed Jan 23, 2026

View reviewed changes

kv2019i requested changes Jan 26, 2026

View reviewed changes

ranj063 added 2 commits January 27, 2026 08:22

module: Add support for module-specific init data

5837ec3

Add a global ID for module specific init data in the extended init data section and update the decode function to save the data in the module's init_data. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 force-pushed the topic/ipc4_compress_part1 branch from 721418e to 9a7f62d Compare January 27, 2026 16:36

ranj063 requested review from Copilot, kv2019i, lgirdwood, lyakh and wjablon1 January 27, 2026 16:37

Copilot AI reviewed Jan 27, 2026

View reviewed changes

ranj063 force-pushed the topic/ipc4_compress_part1 branch from 9a7f62d to dd9f7f2 Compare January 27, 2026 16:54

Compressed offload support for IPC4 #10492

Are you sure you want to change the base?

Compressed offload support for IPC4 #10492

Uh oh!

Conversation

ranj063 commented Jan 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

ranj063 commented Jan 21, 2026

Uh oh!

dbaluta commented Jan 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranj063 Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranj063 Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ranj063 commented Jan 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ranj063 commented Jan 27, 2026

Uh oh!

Reviewers

Assignees

Labels

ranj063 Jan 23, 2026 •

edited

Loading

ranj063 Jan 27, 2026 •

edited

Loading